JIT: Exclude BB_UNITY_WEIGHT scaling from BasicBlock::isBBWeightCold
#116548
Conversation
github-actions
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull Request Overview
This PR adjusts BasicBlock::isBBWeightCold to compare the raw block weight against the product of call count and the cold-weight threshold, removing the previous unity-weight normalization.
- Swaps
getBBWeight(comp)forbbWeight < (getCalledCount(comp) * BB_COLD_WEIGHT) - Aims to restore the intended 1% cutoff for cold blocks rather than 0.01%
Comments suppressed due to low confidence (2)
src/coreclr/jit/block.h:1309
- There are no tests covering the updated
isBBWeightColdbehavior; consider adding unit tests for blocks exactly at, just below, and just above the computed cold threshold.
return bbWeight < (getCalledCount(comp) * BB_COLD_WEIGHT);
src/coreclr/jit/block.h:1309
- The new logic may misclassify blocks when
getCalledCount(comp)returns zero, causing all blocks to appear hot. Consider guarding against a zero call count or falling back to a default behavior for unknown call counts.
return bbWeight < (getCalledCount(comp) * BB_COLD_WEIGHT);
|
cc @dotnet/jit-contrib, @AndyAyersMS PTAL. Diffs have quite a bit of churn in both directions, though I'm taking solace in the relatively small PerfScore diffs, since this primarily affects the placement of cold code. I expect this to be a notable regression only in cases where we mistakenly give hot blocks near-zero weight, and we were likely already doing a subpar job with such cases (example). Thanks! |
|
ping @AndyAyersMS |
|
/ba-g unrelated wasm build failure |
Motivating example.
BB_COLD_WEIGHTis currently 0.01, suggesting blocks that execute less than 1% of the time on average should be considered cold by block layout. However,BasicBlock::isBBWeightColdcompares the block's normalized weight (withBB_UNITY_WEIGHTscaling) againstBB_COLD_WEIGHT, so the actual cutoff for cold blocks is currently 0.01%. This change removes theBB_UNITY_WEIGHTscaling to get the intended behavior.